Fix ffmpeg_reader: reshape order, frame-success return, and tuple check#1107
Open
ds17f wants to merge 1 commit into
Open
Fix ffmpeg_reader: reshape order, frame-success return, and tuple check#1107ds17f wants to merge 1 commit into
ds17f wants to merge 1 commit into
Conversation
The ffmpeg recorder had three bugs that prevented it from supplying usable frames to the recognition pipeline: 1. `numpy.reshape([-1, self.width, self.height, 3])` swaps the spatial dimensions. ffmpeg's rgb24 output is row-major (height, width, 3); reshaping with width as the row axis transposes every frame. 2. `read()` returned (0, ...) on the success paths. Callers in compare.py and add.py treat the first return value as a boolean success flag (matching cv2.VideoCapture.read()), so successful frames were always discarded. 3. `if self.video == ():` becomes ambiguous once self.video is a numpy array — equality returns an element-wise array and triggers 'truth value of an array is ambiguous'. `isinstance(..., tuple)` matches the pre-record sentinel without depending on equality. With these three fixes the ffmpeg recording_plugin produces usable frames; without them, switching from opencv to ffmpeg silently breaks recognition.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The ffmpeg recorder has three bugs that prevent it from supplying usable frames to the recognition pipeline. With these fixes, switching
recording_plugin = ffmpegactually works; without them, it silently breaks recognition.The bugs
1. Reshape transposes spatial dimensions
ffmpeg's
rgb24output is row-major:(height, width, 3). Usingwidthas the row axis transposes every frame.2. Successful reads return
0read()returns(0, ...)on its success paths. Callers incompare.pyandadd.pytreat the first value as a boolean success flag (matchingcv2.VideoCapture.read()), so successful frames are always discarded.3.
if self.video == ():becomes ambiguous after the first recordOnce
self.videois a numpy array, equality returns an element-wise array and triggersValueError: The truth value of an array with more than one element is ambiguous.This is exactly the failure reported in #835:
isinstance(self.video, tuple)matches the pre-record sentinel without depending on equality.Scope
recording_plugin = ffmpeginconfig.ini. Default (opencv) is unchanged.Fixes #835.